-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use API for bypass-checks
#3629
Conversation
🔥 |
Co-authored-by: Federico <me@fregante.com>
This comment has been minimized.
This comment has been minimized.
source/features/bypass-checks.tsx
Outdated
detailsLink.href, | ||
'[data-hydro-click*="check_suite.external_click"]' | ||
); | ||
const getDirectLink = mem(async (runId: number): Promise<string> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API is already memoized, no need to do it again. You can probably just inline it like it was
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep doing it since I remember from here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be removed from there too. It would only make sense if it saved heavy operations (e.g. HTTP requests), but what follows is just a few conditions.
source/features/bypass-checks.tsx
Outdated
detailsLink.href = directLink.href; | ||
} | ||
const runId = new URLSearchParams(detailsLink.search).get('check_run_id') ?? detailsLink.pathname.split('/').pop(); | ||
const directLink = await api.v3(`repos/${getRepoURL()}/check-runs/${Number(runId)}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why number? It goes into a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I have no idea why I did that!
LINKED ISSUES:
Dont remember which issue
TEST URLS:
Enable automatic annotations on GitHub Actions xojs/xo#497
Do not throw on custom stack traces sindresorhus/got#1491
feat(Stats): recursively get warnings or errors of a stats and its children webpack/webpack#11438
SCREENSHOT:
None